-
Notifications
You must be signed in to change notification settings - Fork 149
refactor: migrate tests to Vitest + TS #1202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
refactor: migrate tests to Vitest + TS #1202
Conversation
…e-ts-refactor-redone
Co-authored-by: j-k <dev@j-k.io> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Co-authored-by: j-k <dev@j-k.io> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Co-authored-by: j-k <dev@j-k.io> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Co-authored-by: j-k <dev@j-k.io> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Co-authored-by: Kris West <kristopher.west@natwest.com> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Co-authored-by: Kris West <kristopher.west@natwest.com> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Co-authored-by: Kris West <kristopher.west@natwest.com> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
…om/jescalada/git-proxy into 1150-vitest-migration-from-service
e46cfac to
4b9b89b
Compare
4b9b89b to
61d349f
Compare
|
@kriswest I think this is ready for another look - hoping to get the v2 release ready soon! |
kriswest
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks for the logging cleanup and email comparison fix. Still had a couple of type overrides I wasn't sure you needed... worth leaving a comment if it is needed.
The only lingering issue I see is the proxy tests. Logging errors on start/stop might be worth doing (as it appears not to have stopped...). If that doesn't turn up the answer then it should go on an issue so we can get this merged.
|
@kriswest Thanks for taking another look! Hoping it's ready to go now 😃 |
Co-authored-by: Kris West <kristopher.west@natwest.com> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Co-authored-by: Kris West <kristopher.west@natwest.com> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>
Fixes #1150, built on top of #1166.
Not all tests were perfectly refactored from Chai/Mocha -> Vitest. Some required partial or total rewriting to get them to play nice.
Some tests, specifically the proxy-related tests (
proxy.test.ts,testProxy.test.tsand most importantlytestProxyRoute.test.ts) have been very problematic for some reason - the tests intestPush.test.tsseem to fail in the CI when some of the proxy tests execute prior. They fail on the first execution locally, and then pass without issue (maybe a caching/cleanup problem).Coverage has dropped somewhat to 80.5%, but we should see an increase after removing unused/untested code such as plugin stuff.
Note that the CI is failing due to the new Blue Oak license.